Add support for selection-based result filtering in result viewer#4362
Add support for selection-based result filtering in result viewer#4362asgerf wants to merge 14 commits intogithub:mainfrom
Conversation
24df5f9 to
86e3e25
Compare
30ccbf3 to
2bca2f8
Compare
There was a problem hiding this comment.
Pull request overview
Adds a selection-based filtering mode to the results viewer. When enabled, the results view is restricted to the currently viewed file (and further restricted to the current editor selection range when the selection is non-empty), with extension ↔ webview messaging to avoid stale/incorrect results.
Changes:
- Introduces a “Filter results to current file or selection” checkbox and a dedicated empty-state message for selection-filtered views.
- Lifts
selectedTablestate toResultsAppand adds new messaging/state to request/receive file-filtered results from the extension. - Implements file/selection filtering helpers for raw rows and SARIF results, and updates UI to show filtered vs total counts and disable pagination while filtering.
Show a summary per file
| File | Description |
|---|---|
| extensions/ql-vscode/src/view/results/SelectionFilterNoResults.tsx | New empty-state UI when selection filtering yields no matches. |
| extensions/ql-vscode/src/view/results/SelectionFilterCheckbox.tsx | New checkbox control to toggle selection/file filtering. |
| extensions/ql-vscode/src/view/results/ResultTablesHeader.tsx | Adds pagination-disable mode while selection filtering is active. |
| extensions/ql-vscode/src/view/results/ResultTables.tsx | Wires checkbox state, disables pagination, computes filtered rows/results and filtered counts. |
| extensions/ql-vscode/src/view/results/ResultTable.tsx | Applies filtered rows/results to child tables and shows a filter-specific no-results message. |
| extensions/ql-vscode/src/view/results/ResultsApp.tsx | Lifts selected table state; requests/receives file-filtered results; tracks editor selection/filter state. |
| extensions/ql-vscode/src/view/results/ResultCount.tsx | Displays filtered/total result counts when filtering is active. |
| extensions/ql-vscode/src/view/results/result-table-utils.ts | Adds raw-row and SARIF filtering utilities and selection overlap logic. |
| extensions/ql-vscode/src/local-queries/results-view.ts | Sends editor selection updates; handles file-filtered results requests (BQRS decode + SARIF filtering). |
| extensions/ql-vscode/src/databases/local-databases/locations.ts | Returns revealed editor/location from jump helpers to support selection updates after navigation. |
| extensions/ql-vscode/src/common/sarif-utils.ts | Adds helpers to normalize file URIs and extract all locations from SARIF results. |
| extensions/ql-vscode/src/common/interface-types.ts | Adds shared types/messages for editor selection and file-filtered results. |
| extensions/ql-vscode/CHANGELOG.md | Documents the new selection-based filtering feature. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (4)
extensions/ql-vscode/src/view/results/ResultTables.tsx:172
- When selection filtering disables pagination,
offsetis still computed from the currentpageNumber. If the user enables filtering while on a later page, row numbering (and any offset-based logic inRawTable) will start at a non-zero index even though the view is now showing an unpaged, file-filtered set. Consider usingoffset = 0when pagination is disabled / selection filtering is active.
const offset = parsedResultSets.pageNumber * parsedResultSets.pageSize;
extensions/ql-vscode/src/local-queries/results-view.ts:1204
loadFileFilteredResultsdecodes the entire result set in onebqrsDecodecall usingpageSize: schema.rows. For large result sets this can be extremely slow/memory-heavy even if only a small subset matches the file. Consider decoding incrementally using the pagination offsets/page size (and short-circuit once you’ve collected enough matches for the UI), rather than decoding all rows at once.
if (schema && schema.rows > 0) {
const resultsPath = query.completedQuery.getResultsPath(selectedTable);
const chunk = await this.cliServer.bqrsDecode(
resultsPath,
schema.name,
{
offset: schema.pagination?.offsets[0],
pageSize: schema.rows,
},
);
extensions/ql-vscode/src/local-queries/results-view.ts:1211
- The conditional
if (schema && schema.rows > 0)means tables with zero rows result inrawRowsstayingundefined, which can lead to the webview treating file-filtered results as perpetually “loading” (since no concrete empty result is returned). Consider settingrawRowsto an empty array when the schema exists but has 0 rows (and/or always returning aFileFilteredResultsobject for the requested (fileUri, selectedTable), even if both arrays are empty).
const schema = resultSetSchemas.find((s) => s.name === selectedTable);
if (schema && schema.rows > 0) {
const resultsPath = query.completedQuery.getResultsPath(selectedTable);
const chunk = await this.cliServer.bqrsDecode(
resultsPath,
schema.name,
{
offset: schema.pagination?.offsets[0],
pageSize: schema.rows,
},
);
const resultSet = bqrsToResultSet(schema, chunk);
rawRows = filterRowsByFileUri(resultSet.rows, normalizedFilterUri);
if (rawRows.length > RAW_RESULTS_LIMIT) {
rawRows = rawRows.slice(0, RAW_RESULTS_LIMIT);
}
}
} catch (e) {
extensions/ql-vscode/src/local-queries/results-view.ts:1209
rawRowsis sliced toRAW_RESULTS_LIMITbefore being sent to the webview. This can silently drop matching rows (including ones inside the selection range) and also preventsRawTablefrom showing its existing "Too many results to show at once… omitted" message because it never sees the full length. Consider sending truncation metadata (or collecting up to the limit while still tracking omitted count) so the UI can indicate that results were truncated.
const resultSet = bqrsToResultSet(schema, chunk);
rawRows = filterRowsByFileUri(resultSet.rows, normalizedFilterUri);
if (rawRows.length > RAW_RESULTS_LIMIT) {
rawRows = rawRows.slice(0, RAW_RESULTS_LIMIT);
}
- Files reviewed: 13/13 changed files
- Comments generated: 4
|
Some initial feedback:
|
|
Thanks for looking into this. I'll look into restructuring the commit history when I have time. As for the way filtering work: It started out being more server-side but state management became too error-prone. There's more state to keep track of than is obvious at first, and you end up with a bunch of fields and patterns of "whenever X changes/happens remember to also do Y". I eventually refactored into the tried and true "React + stateless server" pattern which is less error-prone. It's hard to remember exactly why it was so hard, other than the fact that there was always one more nuance to deal with, and it felt like the architecture wasn't sound. This is my best attempt at bringing up from memory what the issues were: Selection and active editor
Race conditions Pagination We'd also have to re-paginate the results whenever the selection changes, preferrably starting using the cached file-filtered results for efficiency. Not impossible to implement or anything, but I wouldn't say we get pagination "for free". |
|
Thanks for the additional context! Sounds like server-side filtering is simpler only in theory; in that case client-side filtering makes sense. Please @-mention me when you are done with the commit restructuring! |
For now the checkbox doesn't do anything, we just wire up the state
Just adds the state and messages, and invalidation logic. No actual filtering happens yet
We disable the UI rather than removing it, as removing it causes layout wobbles when toggling the filter checkbox.
Fixes an issue where: - Filtering is disabled - User clicks a link causing a new editor to open - User enables filtering The editor selection would not previously get updated after step 2, leading to incorrect filtering.
fa298c5 to
babc2b8
Compare
cklin
left a comment
There was a problem hiding this comment.
Thank you for breaking the PR down to smaller commits! Just two minor questions.
| } | ||
| const selection = this.computeEditorSelection(); | ||
| if (selection === undefined) { | ||
| return; |
There was a problem hiding this comment.
I am surprised by the early return here. How would the user revert from selection filtering to whole-file filtering?
| jumpTarget.uri, | ||
| jumpTarget.range, | ||
| ), | ||
| wasFromUserInteraction: false, |
There was a problem hiding this comment.
Should wasFromUserInteraction be true here?
Adds support for selection-based result filtering via a checkbox in the result viewer.
A tuple matches the filter if any of its cells contain an entity whose location matches the filter.
selection-filter-cg.mp4
Implementation notes
The result viewer shows results in pages and the webview only has access to its current page. But if we simply apply the filtering on a page-by-page basis, the filtered results could end being scattered across thousands of mostly-empty pages.
The ideal might have been to support for filtering in
codeql bqrslike we do for sorting, although that wouldn't work for interpreted results from SARIF files. This is the approach I took:The most complex part of the change is getting the extension <-> webview communication to work well and making sure we don't show stale results regardless of what order UI changes occur in.
The
selectedTablestate had to be lifted one component up in the hierarchy (fromResultTabletoResultApp) so it is available at the point where we request file-filtered results from the extension.